@zroubalik I fixed the build

/run-e2e aws
Update: You can check the progress here

/run-e2e aws
Update: You can check the progress here

@zroubalik Is something wrong with the e2e tests? I see they failed twice now :(

@zroubalik Is something wrong with the e2e tests? I see they failed twice now :(

yeah, issue on our side, will rerun once it is fixed

@zroubalik any updates on the e2e testing?

/run-e2e aws
Update: You can check the progress here

@JorTurFer I rebased the PR
Now the tests failed, but it looks like some random crash, can you please rerun the failing job and the e2e tests?

/run-e2e aws
Update: You can check the progress here

Any insight into why it paniced/failed on the amd64 validation?

@JorTurFer is the validate - amd64 stage failure an issue with the action or an issue with the code changes in this pr?

@JorTurFer is the validate - amd64 stage failure an issue with the action or an issue with the code changes in this pr?

I've triggered it again because probably it was a transient error

@zroubalik @wozniakjan PTAL

/run-e2e aws
Update: You can check the progress here

Great catch

i would suggest to add Region in aws authorization metadata, rather than passing it all around.

https://github.com/kedacore/keda/blob/5ea1eac06e536a04cdf22383faff9e91bd374f17/pkg/scalers/aws/aws_authorization.go#L18C1-L35

wdyt @JorTurFer ?

i would suggest to add Region in aws authorization metadata, rather than passing it all around.

5ea1eac/pkg/scalers/aws/aws_authorization.go#L18C1-L35

wdyt @JorTurFer ?

That's a good idea tbh. Could you update the PR @maxbog ?

@JorTurFer @ThaSami
Thanks for the review.
At first glance, I also thought this might be a good idea. However, after digging into the code, I'm not that convinced anymore. Here are the reasons:

1. Although the AuthorizationMetadata struct has Authorization in the name, what it really represents is an AWS identity: a role + credentials coming from pod identity or the AccessKey/SecretAccessKey pair. Identities are global in AWS, so IMO, region does not belong in this struct. Region is actually only used when creating AWS clients, to specify, where the queried services are, not where the identity is located.
2. We would get rid of the awsRegion parameter in the getCacheKey function, but then, if we still want to add it to the AuthorizationMetadata, we need to add a awsRegion parameter to the GetAwsAuthorization function, which, in turn, means we have to pass it from the different types of scaler metadata (for example, here, here, or here, or 5 different places)
3. Having the awsRegion parameter in the getCacheKey actually makes sense when you look at the AuthorizationMetadata as an identity. The aws.Config struct represents a AWS service client configuration, so IMO it makes perfect sense to cache it for a specific identity and for a specific region. We lose expressiveness by hiding the region in the awsAuthorization parameter.

Please let me know, if, after taking the above into account, you still want to update the PR and add the region to the AuthorizationMetadata.

Although the AuthorizationMetadata struct has Authorization in the name, what it really represents is an AWS identity: a role + credentials coming from pod identity or the AccessKey/SecretAccessKey pair.

Exactly - that's what sparked this idea for me. Since credentials are part of identity, obtaining them in the AWS SDK requires specifying a region as STS endpoints are regional.
that's why i believe including AwsRegion within the AuthorizationMetadata struct makes architectural sense because:

1. keeps all authentication-related data in one cohesive unit
2. Ensures the region information is always available when making credential requests
3. maintains a clear relationship between authorization data and its associated AWS region

would like to hear from @JorTurFer